Skip to content
This repository has been archived by the owner on Oct 8, 2021. It is now read-only.

Add greedy_color for Graph Coloring #844

Merged
merged 3 commits into from
Feb 20, 2018

Conversation

SohamTamba
Copy link
Contributor

Fixes #773

Greedy graph coloring algorithm that colors vertices according to some permutation.

I'm not sure if this is the most appropriate folder to place this function.

@codecov
Copy link

codecov bot commented Feb 16, 2018

Codecov Report

Merging #844 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #844   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          58     59    +1     
  Lines        2971   3001   +30     
=====================================
+ Hits         2971   3001   +30


Stores number of colors used and mapping from vertex to color
"""

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no blank lines between docstring and code.

best_color(c1::coloring, c2::coloring) = c1.num_colors < c2.num_colors ? c1 : c2

@doc_str """
Perform greedy coloring [https://en.wikipedia.org/wiki/Greedy_coloring]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs the function signature as the first line in the docstring, indented. The description is not indented:

"""
    GCD(a, b)

Return the greatest common denominator of `a` and `b`.
"""
function GCD(a, b)
  ...
end

@doc_str """
Perform greedy coloring [https://en.wikipedia.org/wiki/Greedy_coloring]

Colors the nodes of an graph g with consecutive integers starting from 1.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use action words here: "Color the nodes of a graph g..."


end

function degree_greedy_color(g::AbstractGraph{U}) where U<:Integer
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs docstring

return result
end

function random_greedy_color(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs docstring

return best
end

function seq_random_greedy_color(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docstring

for graph in [g4, g5]
for g in testgraphs(graph)
for op1 in (true, false), op2 in (true, false)
C = @inferred(greedy_color(g, reps=50, sort_degree=op1, parallel=op2))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice way of doing it :)

@SohamTamba
Copy link
Contributor Author

@sbromberger : I've made the requested changes, please review


for w in neighbors(g, v)
if seen[w]
colors_used[Int(cols[w])] = true
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need to cast this as Int.

end

result = coloring{T}(maximum(cols), cols)
return result
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just do return coloring{T}(maximum(cols), cols) here.

Color graph `g` iteratively in the descending order of the degree of the vertices.
"""
function degree_greedy_color(g::AbstractGraph{U}) where U<:Integer
seq::Vector{U} = sortperm(degree(g, vertices(g)) , rev=true)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

degree(g) will give you a vector of all degrees indexed by vertex. No need for vertices(g) here. Also, you don't need to type assert seq.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing type assert gives an error because sortperm outputs Array{Int64}

Copy link
Owner

@sbromberger sbromberger Feb 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to convert, do so explicitly:

seq = convert(Vector{U}, sortperm(degree(g), rev=true))

function degree_greedy_color(g::AbstractGraph{U}) where U<:Integer
seq::Vector{U} = sortperm(degree(g, vertices(g)) , rev=true)
result = perm_greedy_color(g, seq)
return result
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return perm_greedy_color(g, seq)

)

best::coloring = @parallel (best_color) for i in 1:reps
seq = shuffle(vertices(g))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may not be an issue, but check threads / forums for cautions regarding randomness in parallel.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In JuliaLang/julia#94 it states that each processor starts with different seeds. Will that be sufficient for this problem?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can confirm we get different seeds, then sure. I just raised it as a potential issue based on what I remember reading on discourse / issues. Probably not worth worrying about.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked the seeds on my computer on obtained these results.
julia> @sync @parallel for i in 1:5
println(Base.Random.GLOBAL_RNG.seed)
end
From worker 2: UInt32[0x8dca2c1e, 0xf8d0c0c2, 0x41bd9aef, 0x8f93d3b6]
From worker 2: UInt32[0x8dca2c1e, 0xf8d0c0c2, 0x41bd9aef, 0x8f93d3b6]
From worker 3: UInt32[0xdd8a8792, 0x85d04ba8, 0xc03a6442, 0x7ee40770]
From worker 5: UInt32[0x9898716c, 0x6206fe0f, 0x4fc67721, 0xfc8ace37]
From worker 4: UInt32[0xdf5c2a4b, 0xae7c907a, 0x275310b2, 0xbc04a35b]

It think its safe to confirm that different processes have different seeds.

)

seq = shuffle(vertices(g))
best::coloring = perm_greedy_color(g, seq)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for type assertion here.

return seq_random_greedy_color(g, reps)
else
return parallel_random_greedy_color(g, reps)
end
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return parallel ? parallel_random_greedy_color(g, reps) : seq_random_greedy_color(g, reps)

reps::Integer=1,
)::coloring{U} where U <: Integer

result = coloring(zero(U), zeros(U, nv(g)))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are you assigning result to a value here when you're overwriting it below?

end
return result
end

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function can be written as a one-liner:

greedy_color(
     g::AbstractGraph{U};
     sort_degree::Bool=false,
     parallel::Bool = false,
     reps::Integer=1,
 ) = sort_degree ? degree_greedy_color(g) : random_greedy_color(g, reps, parallel)

C.colors[Int(src(e))] == C.colors[Int(dst(e))] && (correct = false)
end

@test correct
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this test fails, we won't know which component failed. Better to @test explicitly on lines 21 and 23.

@sbromberger
Copy link
Owner

@SohamTamba not to rush this, but we're going to be going into 0.6 feature freeze soon. It might be nice to have this working in Julia 0.6 but it's certainly not necessary. Let me know your preference.

@SohamTamba
Copy link
Contributor Author

SohamTamba commented Feb 19, 2018

I'm almost done.
I'm having trouble finding out whether randomness in parallel will be an issue.

Also I was working on Johnson's all pairs shortest path algorithm earlier. I'll try to finish that soon too.

@SohamTamba
Copy link
Contributor Author

SohamTamba commented Feb 20, 2018

@sbromberger
I made the requested changes in the latest commit and confirmed that different processes have different seeds.

@sbromberger sbromberger merged commit dc5c443 into sbromberger:master Feb 20, 2018
@sbromberger
Copy link
Owner

Nice work - thanks, @SohamTamba !

@SohamTamba SohamTamba mentioned this pull request Mar 10, 2018
@SohamTamba SohamTamba deleted the coloring branch May 18, 2018 19:37
matbesancon added a commit to matbesancon/LightGraphs.jl that referenced this pull request Jun 5, 2018
* removed flow algorithms (sbromberger#815)

* fixes sbromberger#820, plus tests (sbromberger#821)

* change show() for empty graphs (ref JuliaGraphs/MetaGraphs.jl#20 (comment)) (sbromberger#822)

* Pull request clique_percolation.jl (sbromberger#826)

clique percolation is a method of uncovering the overlapping community structure of complex networks in nature and society

* add src/community/clique_percolation.jl
* tests in file test/community/clique_percolation.jl
* cites the original clique percolation paper
* for undirected graphs only using traitfn

* in_ / out_ -> in / out (sbromberger#830)

* (in,out)_neighbors -> (in,out)neighbors

* all_neighbors -> allneighbors

* Pull request clique_percolation.jl (sbromberger#826)

clique percolation is a method of uncovering the overlapping community structure of complex networks in nature and society

* add src/community/clique_percolation.jl
* tests in file test/community/clique_percolation.jl
* cites the original clique percolation paper
* for undirected graphs only using traitfn

* revert allneighbors

* expected_degree_graph (Chung-Lu model) (sbromberger#832)

* Expected degree random graph generator implemented, including tests

* algorithm corrected

* Missing seed corrected in expected_degree_graph

* expected_degree_graph! implemented

* Added return in function, comment with references removed, references in docs added (expected_degree_graph)

* Update randgraphs.jl

minor doc update

* Update randgraphs.jl

* Fixing problems with MST Kruskal's on SimpleWeightedGraphs (sbromberger#835)

* Update kruskal.jl

* Update prim.jl

* Update kruskal.jl

* Update kruskal.jl

* Update prim.jl

* Update kruskal.jl

* Update prim.jl

* Update kruskal.jl

* Update prim.jl

* reverting changes

* Revert "reverting changes"

This reverts commit ac1760b.

* Revert "Update prim.jl"

This reverts commit 677f6fa.

* Revert "Update kruskal.jl"

This reverts commit a0e9c47.

* Revert "Update prim.jl"

This reverts commit 793bac4.

* Revert "Update kruskal.jl"

This reverts commit 6114e16.

* Revert "Update prim.jl"

This reverts commit 551f1e6.

* Revert "Update kruskal.jl"

This reverts commit 941005e.

* Revert "Update kruskal.jl"

This reverts commit a404514.

* Revert "Update prim.jl"

This reverts commit 2d43a60.

* Revert "Update kruskal.jl"

This reverts commit 4577920.

* fix problems with SimpleWeightedGraphs

* fix problems with SimpleWeightedGraphs

* fix problems with SimpleWeightedGraphs

* bipartite_map on 2-order graphs fixed. Added proper tests. Fixed test connected to bipartite_map (sbromberger#836)

* Correct pre-allocation of memory in Prim's MST (sbromberger#839)

* Improve Kruskal's MST by optimizing Union-Find (sbromberger#843)

* add missing backtick (sbromberger#846)

* Add greedy_color for Graph Coloring (sbromberger#844)

* Add greedy_color for Graph Coloring

* Improve Kruskal's MST by optimizing Union-Find (sbromberger#843)

* Update README.md

* Update README.md

* first cut at 0.7 compatibility (sbromberger#848)

* using LightGraphs does not error

* Switch to LinearAlgebra and SparseArrays std libs

* Fix most of linalg tests

* Add SharedArrays for distance tests to compile

* Add Random and Markdown to stdlibs used

* Fix connectivity tests

* IntSet -> BitSet

* Add DelimitedFiles stdlib for readcsv

* Fix failing test

* first cut

* Use mauro/SimpleTraits.jl/m3/where-fns in tests

* Fix SimpleTraits checkout (sbromberger#851)

* Move up SimpleTraits checkout (sbromberger#852)

* Update runtests.jl

* Update REQUIRE

* Update REQUIRE

* femtocleaner with additional depwarn fixes (sbromberger#856)

fix deprecation warnings based on local femtocleaner run

* use equalto in degeneracy.jl (sbromberger#858)

* fix depwarns in linalg submodule (sbromberger#860)

* update linalg/spectral to fix deprecations

* fix depwarns in graphmatrices

* fixes doc deprecation warnings (sbromberger#861)

* fixes doc deprecation warnings

* adding Base64 to runtests

* Update README.md

* remove add/remove vertex/edge from core, minor bug fix (sbromberger#862)

* remove add/remove vertex/edge from core, minor bug fix

* fix tests

* export add/rem vertex

* remove long-term deprecation warnings (sbromberger#863)

* uninitialized -> undef, blkdiag -> blockdiag, and removed import of d… (sbromberger#865)

* uninitialized -> undef, blkdiag -> blockdiag, and removed import of deprecated functions from LG to LinAlg

* test coverage for digraph eltype

* removes equalto (sbromberger#867)

* optional sorting algorithm for gdistances (sbromberger#868)

add the ability to pass RadixSort to gdistances!

* update url and mention directed graphs explicilty (sbromberger#837)

* update url and mention directed graphs explicilty

* Update graphtypes.md

* Update graphtypes.md

fixed references.

* Speed improvements for function transitiveclosure! (sbromberger#870)

* Speed improvements for function transitiveclosure!

Instead of checking for all paths i -> k and k -> j for a given vertex k
we only iterate over the in- and outneighbours of k.

* Merged some conditionals into a single statement

* Cache efficient Floyd Warshall (sbromberger#873)

* Update floyd-warshall.jl

* Cache efficient Floyd Warshall

* fixed an error where smallgraph(:frucht) had 20 vertices instead of 12 (sbromberger#878)

* Delete .Rhistory

* Added function transitivereduction (sbromberger#877)

* added function transitivereduction

* Update transitivity.jl

docstring formatting

* Fixed some tests && added testdigraphs for all tests

* Johnson Shortest Path for Sparse Graphs (sbromberger#884)

* Johnson Shortest Path for Sparse Graphs

Johnson Shortest Path for Sparse Graphs

* Improved memory efficiency if distmx is mutable

* Improved memory efficiency for parallel implementation

* Update index.md

* Added constructors to create graphs from a vector or an iterator of edges (sbromberger#880)

* Added constructors to create SimpleGraphs and SimpleDiGraphs from a vector or an iterator of edges

* Added constructors to create SimpleGraphs and SimpleDiGraphs from a vector or an iterator of edges

* Slyles1001/892 (sbromberger#894)

* comments are your friend

* Most of LightGraphs warnings are fixed

* Delete HITS.jl

* Slyles1001/872 (sbromberger#891)

* DataStructures fixed

* missed heappop!, now it tests clean

* spaces

* Update LightGraphs.jl

* Update runtests.jl

* fixes most depwarns as of 20180529 (sbromberger#895)

* fixes most depwarns as of 20180529

* graphmatrices problems

* remove tabs

* tabs, again

* Update CONTRIBUTING.md (sbromberger#897)

* Improve Kruskal and use in-built disjoint set data structure (sbromberger#896)

* Improve Kruskal and use in-built disjoint set data structure

* Update kruskal.jl

Changes requested by @somil55
sbromberger pushed a commit that referenced this pull request Jun 14, 2018
* ignore coverage file

* merge master (#2)

* removed flow algorithms (#815)

* fixes #820, plus tests (#821)

* change show() for empty graphs (ref JuliaGraphs/MetaGraphs.jl#20 (comment)) (#822)

* Pull request clique_percolation.jl (#826)

clique percolation is a method of uncovering the overlapping community structure of complex networks in nature and society

* add src/community/clique_percolation.jl
* tests in file test/community/clique_percolation.jl
* cites the original clique percolation paper
* for undirected graphs only using traitfn

* in_ / out_ -> in / out (#830)

* (in,out)_neighbors -> (in,out)neighbors

* all_neighbors -> allneighbors

* Pull request clique_percolation.jl (#826)

clique percolation is a method of uncovering the overlapping community structure of complex networks in nature and society

* add src/community/clique_percolation.jl
* tests in file test/community/clique_percolation.jl
* cites the original clique percolation paper
* for undirected graphs only using traitfn

* revert allneighbors

* expected_degree_graph (Chung-Lu model) (#832)

* Expected degree random graph generator implemented, including tests

* algorithm corrected

* Missing seed corrected in expected_degree_graph

* expected_degree_graph! implemented

* Added return in function, comment with references removed, references in docs added (expected_degree_graph)

* Update randgraphs.jl

minor doc update

* Update randgraphs.jl

* Fixing problems with MST Kruskal's on SimpleWeightedGraphs (#835)

* Update kruskal.jl

* Update prim.jl

* Update kruskal.jl

* Update kruskal.jl

* Update prim.jl

* Update kruskal.jl

* Update prim.jl

* Update kruskal.jl

* Update prim.jl

* reverting changes

* Revert "reverting changes"

This reverts commit ac1760b.

* Revert "Update prim.jl"

This reverts commit 677f6fa.

* Revert "Update kruskal.jl"

This reverts commit a0e9c47.

* Revert "Update prim.jl"

This reverts commit 793bac4.

* Revert "Update kruskal.jl"

This reverts commit 6114e16.

* Revert "Update prim.jl"

This reverts commit 551f1e6.

* Revert "Update kruskal.jl"

This reverts commit 941005e.

* Revert "Update kruskal.jl"

This reverts commit a404514.

* Revert "Update prim.jl"

This reverts commit 2d43a60.

* Revert "Update kruskal.jl"

This reverts commit 4577920.

* fix problems with SimpleWeightedGraphs

* fix problems with SimpleWeightedGraphs

* fix problems with SimpleWeightedGraphs

* bipartite_map on 2-order graphs fixed. Added proper tests. Fixed test connected to bipartite_map (#836)

* Correct pre-allocation of memory in Prim's MST (#839)

* Improve Kruskal's MST by optimizing Union-Find (#843)

* add missing backtick (#846)

* Add greedy_color for Graph Coloring (#844)

* Add greedy_color for Graph Coloring

* Improve Kruskal's MST by optimizing Union-Find (#843)

* Update README.md

* Update README.md

* first cut at 0.7 compatibility (#848)

* using LightGraphs does not error

* Switch to LinearAlgebra and SparseArrays std libs

* Fix most of linalg tests

* Add SharedArrays for distance tests to compile

* Add Random and Markdown to stdlibs used

* Fix connectivity tests

* IntSet -> BitSet

* Add DelimitedFiles stdlib for readcsv

* Fix failing test

* first cut

* Use mauro/SimpleTraits.jl/m3/where-fns in tests

* Fix SimpleTraits checkout (#851)

* Move up SimpleTraits checkout (#852)

* Update runtests.jl

* Update REQUIRE

* Update REQUIRE

* femtocleaner with additional depwarn fixes (#856)

fix deprecation warnings based on local femtocleaner run

* use equalto in degeneracy.jl (#858)

* fix depwarns in linalg submodule (#860)

* update linalg/spectral to fix deprecations

* fix depwarns in graphmatrices

* fixes doc deprecation warnings (#861)

* fixes doc deprecation warnings

* adding Base64 to runtests

* Update README.md

* remove add/remove vertex/edge from core, minor bug fix (#862)

* remove add/remove vertex/edge from core, minor bug fix

* fix tests

* export add/rem vertex

* remove long-term deprecation warnings (#863)

* uninitialized -> undef, blkdiag -> blockdiag, and removed import of d… (#865)

* uninitialized -> undef, blkdiag -> blockdiag, and removed import of deprecated functions from LG to LinAlg

* test coverage for digraph eltype

* removes equalto (#867)

* optional sorting algorithm for gdistances (#868)

add the ability to pass RadixSort to gdistances!

* update url and mention directed graphs explicilty (#837)

* update url and mention directed graphs explicilty

* Update graphtypes.md

* Update graphtypes.md

fixed references.

* Speed improvements for function transitiveclosure! (#870)

* Speed improvements for function transitiveclosure!

Instead of checking for all paths i -> k and k -> j for a given vertex k
we only iterate over the in- and outneighbours of k.

* Merged some conditionals into a single statement

* Cache efficient Floyd Warshall (#873)

* Update floyd-warshall.jl

* Cache efficient Floyd Warshall

* fixed an error where smallgraph(:frucht) had 20 vertices instead of 12 (#878)

* Delete .Rhistory

* Added function transitivereduction (#877)

* added function transitivereduction

* Update transitivity.jl

docstring formatting

* Fixed some tests && added testdigraphs for all tests

* Johnson Shortest Path for Sparse Graphs (#884)

* Johnson Shortest Path for Sparse Graphs

Johnson Shortest Path for Sparse Graphs

* Improved memory efficiency if distmx is mutable

* Improved memory efficiency for parallel implementation

* Update index.md

* Added constructors to create graphs from a vector or an iterator of edges (#880)

* Added constructors to create SimpleGraphs and SimpleDiGraphs from a vector or an iterator of edges

* Added constructors to create SimpleGraphs and SimpleDiGraphs from a vector or an iterator of edges

* Slyles1001/892 (#894)

* comments are your friend

* Most of LightGraphs warnings are fixed

* Delete HITS.jl

* Slyles1001/872 (#891)

* DataStructures fixed

* missed heappop!, now it tests clean

* spaces

* Update LightGraphs.jl

* Update runtests.jl

* fixes most depwarns as of 20180529 (#895)

* fixes most depwarns as of 20180529

* graphmatrices problems

* remove tabs

* tabs, again

* Update CONTRIBUTING.md (#897)

* Improve Kruskal and use in-built disjoint set data structure (#896)

* Improve Kruskal and use in-built disjoint set data structure

* Update kruskal.jl

Changes requested by @somil55

* updated syntax for iterator protocol

* fixed iterator, broken inference
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants